BUG: fix to_json on period #61203
BUG: fix to_json on period #61203
Conversation
xiaohuanlin
force-pushed
the
fix_55490
branch
2 times, most recently
from
24e5d75 to
dd9e2ce
Compare
xiaohuanlin
changed the title
pandas/_libs/tslibs/offsets.pyx
Outdated
| @@ -568,6 +568,19 @@ cdef class BaseOffset: | |||
| out += ": " + ", ".join(attrs) | |||
| return out | |||
|
|
|||
| def toDict(self) -> dict: | |||
There was a problem hiding this comment.
I'd prefer not to implement this here but rather in the C extension code itself; this way of making something JSON serializable takes advantage of an obscure ultrajson method of allowing custom objects to be serialized, and it will be pretty slow compared to direct implementation in our C extension
There was a problem hiding this comment.
Yes, I agree with you on implementing it in C extension code
pandas/tests/io/json/test_pandas.py
Outdated
| # GH55490 | ||
| ser = Series(pd.period_range(start=2021, freq="Y", periods=1)) | ||
| result = ser.to_json() | ||
| expected = ( |
There was a problem hiding this comment.
Is this really the JSON representation that users want for a period?
There was a problem hiding this comment.
I found it interesting that the Period class actually has a freqstr property. This allows us to ignore freq when running to_json, which makes sense since freqstr already contains the necessary frequency information. Additionally, this approach helps resolve the issue.
xiaohuanlin
force-pushed
the
fix_55490
branch
2 times, most recently
from
02739a2 to
07cf8a5
Compare
|
Does anyone else want to review it? |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.